Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enterprise Search] Support for starting ELSER model deployment #156080

Merged
merged 19 commits into from
May 1, 2023

Conversation

demjened
Copy link
Contributor

@demjened demjened commented Apr 27, 2023

Summary

We're adding action buttons to the ELSER model deployment panel, specifically to the state where the trained model has been downloaded but not started yet. The user has two options here:

  • Start the model (synchronously) with a basic configuration using the "Start single-threaded" button
  • Navigate to the Trained Models page and fine-tune the model deployment with the "Fine-tune performance" button

In addition a 4th state of the panel is being introduced: the ELSER model has started.

ELSER_start

Checklist

@demjened demjened added release_note:feature Makes this part of the condensed release notes Team:EnterpriseSearch v8.8.0 labels Apr 27, 2023
@demjened demjened marked this pull request as ready for review April 27, 2023 19:35
@demjened demjened requested a review from a team April 27, 2023 19:35
@julianrosado
Copy link
Contributor

  • Need between paragraph and action buttons (giving it a 24px space)
  • Is there a reason why the title icon and text needs to be black? In a success callout it should be green
  • I know this might be out of scope, but I am still curious if there's any way we can let users know their models are running in a non-ideal but dev ready state (single-threaded)?

@demjened
Copy link
Contributor Author

Need between paragraph and action buttons (giving it a 24px space)

Ok, will update.

Is there a reason why the title icon and text needs to be black? In a success callout it should be green

I'm actually using EuiPanel but I think it should behave the same way. The culprit seems to be FormattedMessage inside an EuiTitle, I'm not sure how to color these because neither of them takes a color attribute. Maybe @TattdCodeMonkey or @sphilipse knows?
For reference this is what it's supposed to look like according to the design:

Screenshot 2023-04-27 at 4 00 17 PM

I know this might be out of scope, but I am still curious if there's any way we can let users know their models are running in a non-ideal but dev ready state (single-threaded)?

My plan is to make these critical state transitions safe, then I'll play around with the different use cases. You're talking about the cases when the user started with the Start button vs did a deployment on multiple threads?

@julianrosado
Copy link
Contributor

Ok, will update.

You da man

I'm actually using EuiPanel but I think it should behave the same way.

Ah I see, I was anticipating EuiCallout which does that auto-magically. I don't think Title accept color params. If it's not a simple change don't waste brain cycles on it.

You're talking about the cases when the user started with the Start button vs did a deployment on multiple threads?

Yeah, if you started it single-threaded just to get development going, but then forget to change it to multi threaded for production specifically because we don't surface that information somewhere useful in this screen. If that is outside the scope of this release that's fine but I'd want to have something for that moving forward.

import { Actions, createApiLogic } from '../../../../shared/api_logic/create_api_logic';
import { HttpLogic } from '../../../../shared/http';

export type StartTextExpansionModelArgs = undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick(non-blocking): Best practice is to specify an empty object rather than undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is breaking the type checks:

/var/lib/buildkite-agent/builds/kb-n2-2-spot-a440796eab9fe2c4/elastic/kibana-pull-request/kibana/x-pack/plugins/enterprise_search/public/applications/enterprise_search_content/api/ml_models/text_expansion/create_text_expansion_model_api_logic.ts
--
  | 2023-04-28 11:19:48 EDT | 11:18  error  An empty interface is equivalent to `{}`  @typescript-eslint/no-empty-interface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sphilipse I reverted to undefined because I couldn't find a way to fix the CI error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what you want to do instead is use {} wherever you're using StartTextExpansionModelArgs. But this is a nitpick anyway.

@demjened demjened requested a review from sphilipse April 30, 2023 23:00
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing everything! Looks good to me.

@demjened
Copy link
Contributor Author

demjened commented May 1, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 2077 2078 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.3MB 2.3MB +2.8KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 479 482 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@demjened demjened merged commit 5897708 into elastic:main May 1, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 1, 2023
…tic#156080)

## Summary

We're adding action buttons to the ELSER model deployment panel,
specifically to the state where the trained model has been downloaded
but not started yet. The user has two options here:
- Start the model (synchronously) with a basic configuration using the
"Start single-threaded" button
- Navigate to the Trained Models page and fine-tune the model deployment
with the "Fine-tune performance" button

In addition a 4th state of the panel is being introduced: the ELSER
model has started.

![ELSER_start](https://user-images.githubusercontent.com/14224983/234971172-a99917dd-ec55-4df3-acd1-a6b390262104.gif)

### Checklist
- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 5897708)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request May 1, 2023
…#156080) (#156266)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Enterprise Search] Support for starting ELSER model deployment
(#156080)](#156080)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Adam
Demjen","email":"demjened@gmail.com"},"sourceCommit":{"committedDate":"2023-05-01T14:56:18Z","message":"[Enterprise
Search] Support for starting ELSER model deployment (#156080)\n\n##
Summary\r\n\r\nWe're adding action buttons to the ELSER model deployment
panel,\r\nspecifically to the state where the trained model has been
downloaded\r\nbut not started yet. The user has two options here:\r\n-
Start the model (synchronously) with a basic configuration using
the\r\n\"Start single-threaded\" button\r\n- Navigate to the Trained
Models page and fine-tune the model deployment\r\nwith the \"Fine-tune
performance\" button\r\n\r\nIn addition a 4th state of the panel is
being introduced: the ELSER\r\nmodel has
started.\r\n\r\n\r\n![ELSER_start](https://user-images.githubusercontent.com/14224983/234971172-a99917dd-ec55-4df3-acd1-a6b390262104.gif)\r\n\r\n\r\n###
Checklist\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"589770885e87c3209a48a1b5d2b4e30576dc8c2e","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:feature","Team:EnterpriseSearch","v8.8.0","v8.9.0"],"number":156080,"url":"#156080
Search] Support for starting ELSER model deployment (#156080)\n\n##
Summary\r\n\r\nWe're adding action buttons to the ELSER model deployment
panel,\r\nspecifically to the state where the trained model has been
downloaded\r\nbut not started yet. The user has two options here:\r\n-
Start the model (synchronously) with a basic configuration using
the\r\n\"Start single-threaded\" button\r\n- Navigate to the Trained
Models page and fine-tune the model deployment\r\nwith the \"Fine-tune
performance\" button\r\n\r\nIn addition a 4th state of the panel is
being introduced: the ELSER\r\nmodel has
started.\r\n\r\n\r\n![ELSER_start](https://user-images.githubusercontent.com/14224983/234971172-a99917dd-ec55-4df3-acd1-a6b390262104.gif)\r\n\r\n\r\n###
Checklist\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"589770885e87c3209a48a1b5d2b4e30576dc8c2e"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#156080
Search] Support for starting ELSER model deployment (#156080)\n\n##
Summary\r\n\r\nWe're adding action buttons to the ELSER model deployment
panel,\r\nspecifically to the state where the trained model has been
downloaded\r\nbut not started yet. The user has two options here:\r\n-
Start the model (synchronously) with a basic configuration using
the\r\n\"Start single-threaded\" button\r\n- Navigate to the Trained
Models page and fine-tune the model deployment\r\nwith the \"Fine-tune
performance\" button\r\n\r\nIn addition a 4th state of the panel is
being introduced: the ELSER\r\nmodel has
started.\r\n\r\n\r\n![ELSER_start](https://user-images.githubusercontent.com/14224983/234971172-a99917dd-ec55-4df3-acd1-a6b390262104.gif)\r\n\r\n\r\n###
Checklist\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"589770885e87c3209a48a1b5d2b4e30576dc8c2e"}}]}]
BACKPORT-->

Co-authored-by: Adam Demjen <demjened@gmail.com>
@demjened demjened deleted the demjened/elser-start-model-deployment branch May 5, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:feature Makes this part of the condensed release notes Team:EnterpriseSearch v8.8.0 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants